-
Notifications
You must be signed in to change notification settings - Fork 376
fix: Remove throwing "initWithContext was not called or timed out", introduced in 5.4.0 #2412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Starting in 5.4.0 calls such as OneSignal.Notification or OneSignal.Location would throw if they waited on the SDK into to long. This comment changes this to wait forever until init is complete, and log instead if the time is taking longer than expected. If these calls were done from the main thread an ANR is the better of two evils and the app can recover, where an uncaught throw it can not. This commit will bring the behavior the similar behavior in 5.1.x. The only difference is you will now never see an ANR with initWithContext, however if you call other parts of the SDK from the main thread, such as OneSignal.Notification, you will continue to see ANRs, just at a different stacktrace. To avoid these ANRs completely call all other SDK methods outside of the main thread.
c386c85 to
00d3380
Compare
|
A comment regarding this, at the SDK level, it's difficult to know that something as simple as Given my lack of context when using the SDK, I initially assume that adding a listener shouldn't require the SDK to be initialized, nor should it need to be done on a non-main thread. And I least expected a failure when there was confirmation that it was initialized. I understand that the fix solves a problem but not the cause. In my opinion, the SDK should either provide an initialization event, perhaps with a listener or something similar, to indicate when it's a good time to subscribe to things. And if certain operations must be done off the main thread, that should be the SDK's default behavior internally, providing options for advanced users to change said behavior. Having said all this, perhaps none of this is necessary if the documentation is supplemented with examples of how to work better with the SDK following these changes. In any case, thank you for working on the fix and constantly improving the SDK. |
Please mark these functions with suspend so that we, as developers, can always see which functions need to be called as suspend functions. |
@alberto-derodrigo-spacego thanks for your feedback here, I agree that this is confusing and appending a listener shouldn't require
The pre-5.4.0 expected pattern was to call
|
|
Description
One Line Summary
Revert throwing if things wait on init take too long that was introduced in 5.4.0.
Details
Starting in 5.4.0 calls such as OneSignal.Notification or OneSignal.Location would throw if they waited on the SDK into to long. This PR changes this to wait forever until init is complete, and log instead if the time is taking longer than expected. If these calls were done from the main thread an ANR is the lesser of two evils and the app can recover, where an uncaught throw it can not.
This commit will bring the behavior similar to 5.1.x. The only difference is you will now never see an ANR with initWithContext, however if you call other parts of the SDK from the main thread, such as OneSignal.Notification, you will continue to see ANRs, just at a different stacktrace. To avoid these ANRs completely call all other SDK methods outside of the main thread.
Related Issues
Motivation
A behavior change wasn't intended in a non-major release.
Scope
Removes throwing and timeout state when things are waiting on SDK initialization.
Testing
Unit testing
Updated existing unit tests to expect waiting without a timeout.
Manual testing
Tested on an Android 14 emulator. Added an artificial 10 second delay with
Thread.Sleepjust beforelatch.countDown()to ensure the SDK still initializes correctly.Affected code checklist
Checklist
Overview
Testing
Final pass
This change is